feat(pageframe): adopt TopBar.Slot in Layout.Title#112515
Conversation
github-actions
bot
added
the
Scope: Frontend
natemoo-re
force-pushed
the
nm/pageframe/title
branch
from
d12e6a7 to
75745e5
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| if (hasPageFrame) { | ||
| return <TopBar.Slot name="title">{props.children}</TopBar.Slot>; | ||
| } |
There was a problem hiding this comment.
Bug: The Layout.Title component in page frame mode discards props like className, which breaks styling applied via styled(Layout.Title).
Severity: MEDIUM
Suggested Fix
Update the Layout.Title component to accept and spread additional props to the underlying element. This will ensure that className and other attributes are passed through correctly.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: static/app/components/layouts/thirds.tsx#L144-L146
Potential issue: In page frame mode, the `Layout.Title` component only forwards its
`children` prop, discarding all other props, including `className`. This prevents styles
applied via `styled(Layout.Title)` from being rendered, causing UI breakages in multiple
locations such as the alerts rule details header and the issue list header.
Did we get this right? 👍 / 👎 to inform future reviews.
| const hasPageFrame = useHasPageFrameFeature(); | ||
|
|
||
| if (hasPageFrame) { | ||
| return <TopBar.Slot name="title">{props.children}</TopBar.Slot>; |
There was a problem hiding this comment.
Bug: A second TopBar.Slot with the name 'title' is registered on Settings pages, causing a conflict that hides either the breadcrumbs or the page title.
Severity: MEDIUM
Suggested Fix
Conditionally render the TopBar.Slot within Layout.Title to avoid registering it on pages where the 'title' slot is already in use, such as the Settings pages. Alternatively, use a different, non-conflicting slot name.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: static/app/components/layouts/thirds.tsx#L145
Potential issue: On Settings pages, which already register a `TopBar.Slot` named 'title'
for breadcrumbs, the `Layout.Title` component unconditionally registers a second
`TopBar.Slot` with the same name. This name collision causes one of the two
elements—either the breadcrumbs or the page title—to be unpredictably hidden from the
top bar, degrading the user experience on settings pages.
Did we get this right? 👍 / 👎 to inform future reviews.
2 participants
Closes DE-1067